Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 94.99%. Comparing base (1dc101b) to head (322ef5f).
Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #3116      +/-   ##
==========================================
+ Coverage   90.45%   94.99%   +4.54%     
==========================================
  Files          84       83       -1     
  Lines        3634     3618      -16     
  Branches     1279     1276       -3     
==========================================
+ Hits         3287     3437     +150     
+ Misses        347      181     -166     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Addressed lint errors

Updated, based on feedback in #3079 to not attempt to avoid the error, but to instead rethrow with a more informative message. I updated the PR description to reflect the new approach. There's not a great way to test this now. With the integration tests before, we could remove the rc config, run lint, and observe that it didn't error out. Now we're wanting it to error out when no rc is present, and there's not a great way to recreate that in the rule's unit tests or in integration tests that don't throw (unless we write a custom node script to serve as an integration test). Thoughts?

I suppose we could have a test make a temp dir (using tmp), copy an example into it, run npm install in it, and then invoke eslint inside it?

I suppose we could have a test make a temp dir (using tmp), copy an example into it, run npm install in it, and then invoke eslint inside it?

Added a test that does the following:

• creates a tmp folder
• copies the v9 example folder into it
• runs exec to install the plugin from cwd and run the lint command
• catches the error and validates that the message includes our new verbiage

@ljharb I got this about where I want it, except for one question to you. I added the test to the existing set of unit tests just because all the CI plumbing was already in place to add it there. Though, it doesn't really feel right for it to be there, since it's more of an integration test. Would you rather it be where I have it now, or add a new workflow to CI for doing this kind of integration testing?

I think that it's fine where it is to land this, but it might make sense to move it in a followup?

… although i do see there's 2 uncovered lines in the diff

I think that it's fine where it is to land this, but it might make sense to move it in a followup?

Sounds good

… although i do see there's 2 uncovered lines in the diff

Do you mean the catch block? If so, that's because the new test isn't really contributing to the coverage report, since it's building the plugin and running lint as a separate exec command.